feat: Client from firebolt apis by tomasz-blasz · Pull Request #7 · rdkcentral/firebolt-cpp-client

tomasz-blasz · 2025-12-11T15:20:48Z

No description provided.

github-actions · 2025-12-11T15:21:01Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

Copilot

Pull request overview

This pull request represents a major refactoring of the Firebolt client SDK, transitioning from the old SDK architecture to a new client-based approach. The PR removes deprecated modules (ClosedCaptions, HDMIInput, SecureStorage, Metrics) and introduces new ones (Accessibility, Advertising, Display, Presentation, Stats) while updating existing modules (Device, Lifecycle, Localization) to align with the new Firebolt APIs. The changes include comprehensive test coverage with both unit and component tests, a new test infrastructure using mock helpers, and updated build configuration.

Key changes include:

Migration from WPEFramework-based transport to a new FireboltTransport layer
Introduction of a helper-based architecture for API calls and subscriptions
Complete test suite reorganization with unit and component test separation
New JSON type handling using nlohmann::json instead of WPEFramework JSON containers

Reviewed changes

Copilot reviewed 95 out of 95 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
test/utils.h, test/utils.cpp	New utility functions for component tests including HTTP helpers and event verification
test/unit/*.cpp	New unit tests for all modules using mock-based testing approach
test/component/*.cpp	New component tests for integration testing with mock server
test/json_engine.h	Test helper for reading expected values from OpenRPC specification
test/unit/mock_helper.h	Mock helper infrastructure for unit testing
test/CMakeLists.txt	Updated test build configuration supporting both unit and component tests
src/_impl.h, src/_impl.cpp	New and updated module implementations using the helper pattern
src/json_types/*.h	JSON type definitions for data serialization/deserialization
src/firebolt.cpp	Updated main accessor with new module initialization
include/firebolt/*.h	Public API headers for all modules
src/CMakeLists.txt	Updated build configuration with export headers and versioning
cmake/FireboltClientConfig.cmake.in	CMake package configuration for library consumers

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

damcav64 · 2025-12-11T15:45:39Z

I have read the CLA Document and I hereby sign the CLA

Copilot

Pull request overview

Copilot reviewed 96 out of 96 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (3)

include/firebolt/firebolt.h:80

There's a typo in "Accessibiilty" (double 'i') which should be "Accessibility".
include/firebolt/firebolt.h:130
There's a typo in "Accessibiilty" (double 'i') which should be "Accessibility".
include/firebolt/firebolt.h:141
There's malformed documentation comment syntax. Lines 135-141 appear to be incorrectly formatted - line 135 has /** followed by line 136 having virtual Accessibility::IAccessibility& AccessibilityInterface() = 0; which looks like leftover code, followed by a proper comment start on line 137. This creates a duplicate AccessibilityInterface() declaration (lines 85 and 136) and malformed comment structure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-11T16:03:20Z

test/component/deviceTest.cpp

+    ASSERT_TRUE(result) << "DeviceImpl::uptime() returned an error";
+    if (expectedValue.empty())
+    {
+        std::cout << "[ !!!      ] Expected is empty, recived: " << *result << std::endl;


There's a typo in "recived" which should be "received".

Copilot · 2025-12-11T16:03:20Z

test/component/deviceTest.cpp

+    ASSERT_TRUE(result) << "DeviceImpl::timeInActiveState() returned an error";
+    if (expectedValue.empty())
+    {
+        std::cout << "[ !!!      ] Expected is empty, recived: " << *result << std::endl;


There's a typo in "recived" which should be "received".

Copilot · 2025-12-11T16:03:21Z

test/unit/deviceTest.cpp

 }

-TEST_F(DeviceTest, Sku)
+TEST_F(DeviceTest, GetClassBadRespons_Test)


There's a typo in the test name "GetClassBadRespons_Test" - it appears "Response" is misspelled as "Respons" (missing the 'e').

Copilot · 2025-12-11T16:03:21Z

test/utils.cpp

+{
+    // Wait for the event to be received or timeout after 5 seconds
+    std::unique_lock<std::mutex> lock(mtx);
+    if (cv.wait_for(lock, std::chrono::seconds(EventWaitTime), [&] { return eventReceived; }))


The conversion from std::chrono::seconds to std::chrono::duration on line 140 is missing template parameters. While this may compile due to implicit conversion, the code on line 29 defines EventWaitTime as constexpr std::chrono::duration which is incomplete - it should specify template parameters like std::chrono::duration<int, std::ratio<1>> or use a specific duration type like std::chrono::seconds.

Copilot

Pull request overview

Copilot reviewed 96 out of 96 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (7)

test/utils.cpp:1

Type mismatch: EventWaitTime is already a std::chrono::duration (std::chrono::seconds), but it's being passed to std::chrono::seconds() constructor again, causing a compilation error. Remove the std::chrono::seconds wrapper.
test/unit/deviceTest.cpp:1
Corrected spelling of 'GetClassBadRespons_Test' to 'GetClassBadResponse_Test'
test/component/deviceTest.cpp:1
Corrected spelling of 'recived' to 'received'
test/component/deviceTest.cpp:1
Corrected spelling of 'recived' to 'received'
src/lifecycle_impl.h:1
Duplicate 'private:' access specifier. Remove one of the duplicate private labels.
src/json_types/jsondata_lifecycle_types.h:1
Using .at() on JSON can throw if the key doesn't exist. Consider validating the presence of 'oldState' and 'newState' keys before accessing them, or provide appropriate error handling for missing keys.
test/json_engine.h:1
Incomplete TODO comment appears to be truncated mid-sentence. Complete the comment or remove it if it's no longer relevant.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 96 out of 96 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-12T08:38:51Z

.github/Dockerfile

 RUN cd /deps \
    && dir="googletest-${DEPS_GOOGLETEST_V}" \
-    && curl -sL https://github.com/google/googletest/releases/download/v${DEPS_GOOGLETEST_V}/$dir.tar.gz | tar xzf - \
+    && i=0 && while ! curl -sL https://github.com/google/googletest/releases/download/v${DEPS_GOOGLETEST_V}/$dir.tar.gz | tar xzf -; do test "$i" -eq 5 && exit 1; i=$((i+1)); sleep 1; done \


This curl | tar pipeline downloads and extracts the googletest source archive during the CI image build without any checksum or signature verification, creating a supply-chain risk. If an attacker can tamper with the GitHub release asset or intercept the connection, they could deliver a malicious archive whose build system executes arbitrary code in your build environment. Fetch the tarball via HTTPS but also pin and verify its integrity (e.g. by checking a known hash or signature) before extraction and compilation.

Copilot · 2025-12-12T08:38:52Z

.github/Dockerfile

 RUN cd /deps \
    && dir="json-schema-validator-${DEPS_JSON_SCHEMA_VALIDATOR_V}" \
-    && curl -sL https://github.com/pboettch/json-schema-validator/archive/refs/tags/${DEPS_JSON_SCHEMA_VALIDATOR_V}.tar.gz | tar xzf - \
+    && i=0 && while ! curl -sL https://github.com/pboettch/json-schema-validator/archive/refs/tags/${DEPS_JSON_SCHEMA_VALIDATOR_V}.tar.gz | tar xzf - ; do test "$i" -eq 5 && exit 1; i=$((i+1)); sleep 1; done \


This curl | tar pipeline for json-schema-validator downloads and unpacks a remote archive without any integrity check, which is a supply-chain vulnerability. A compromised GitHub release or network path could provide a malicious tarball whose CMake or build scripts execute arbitrary code during the Docker image build. Add a pinned hash or signature verification step for the downloaded archive and fail the build if the content does not match the expected value.

Copilot · 2025-12-12T08:38:52Z

.github/Dockerfile

 RUN cd /deps \
    && dir="websocketpp-${DEPS_WEBSOCKETPP_V}" \
-    && curl -sL https://github.com/zaphoyd/websocketpp/archive/refs/tags/${DEPS_WEBSOCKETPP_V}.tar.gz | tar xzf - \
+    && i=0 && while ! curl -sL https://github.com/zaphoyd/websocketpp/archive/refs/tags/${DEPS_WEBSOCKETPP_V}.tar.gz | tar xzf - ; do test "$i" -eq 5 && exit 1; i=$((i+1)); sleep 1; done \


The curl | tar usage for websocketpp pulls and extracts unverified code from GitHub at build time, exposing the CI build to supply-chain compromise. If the tarball is replaced or tampered with, its contents or build configuration could run arbitrary commands when you subsequently configure and build it. Ensure the archive is pinned to an immutable artifact and that its checksum or signature is validated before extraction and compilation.

Copilot · 2025-12-12T08:38:52Z

.github/Dockerfile

 RUN cd /deps \
    && dir="firebolt-native-transport-${DEPS_TRANSPORT_V}" \
-    && curl -sL https://github.com/rdkcentral/firebolt-native-transport/releases/download/v${DEPS_TRANSPORT_V}/firebolt-native-transport-${DEPS_TRANSPORT_V}.tar.gz | tar xzf - \
+    && i=0 && while ! curl -sL https://github.com/rdkcentral/firebolt-native-transport/releases/download/v${DEPS_TRANSPORT_V}/firebolt-native-transport-${DEPS_TRANSPORT_V}.tar.gz | tar xzf - ; do test "$i" -eq 5 && exit 1; i=$((i+1)); sleep 1; done \


This curl | tar pipeline for firebolt-native-transport downloads and extracts a remote release without verifying its integrity, which is a direct supply-chain risk. An attacker who can alter the release asset or intercept the download could deliver a malicious archive whose build steps execute arbitrary code in your CI environment, potentially accessing secrets or poisoning artifacts. Add integrity verification (e.g. pinned hash or signature check) for the downloaded tarball and fail the build if the content is not exactly what is expected.

Copilot · 2025-12-12T08:38:52Z

CMakeLists.txt

+    FetchContent_Declare(
+        FireboltTransport
+        URL "https://github.com/rdkcentral/firebolt-native-transport/releases/download/v${FIREBOLT_TRANSPORT_VERSION}/firebolt-native-transport-${FIREBOLT_TRANSPORT_VERSION}.tar.gz"
+        DOWNLOAD_EXTRACT_TIMESTAMP true
+    )
+    set(ENABLE_TESTS_ORIG ${ENABLE_TESTS})
+    set(ENABLE_TESTS OFF)
+    FetchContent_MakeAvailable(FireboltTransport)


FetchContent_Declare is downloading and building FireboltTransport directly from a GitHub release tarball without any integrity verification, which introduces a supply-chain risk. If the remote artifact or the network path is compromised, a malicious archive could inject arbitrary code into your build via its build scripts or sources. Pin this dependency to an immutable artifact and enforce a checksum/signature check (e.g. via URL_HASH or an equivalent mechanism) so the configure step fails on tampering.

…, fix failing jobs (#:412)

* feat: Add component tests for APIs except events * refactor: Refactor FindCurl.cmake

* build: Set project version from release pipeline or tag * style: change include * test: fix gtest_discover_tests * ci: Update ci pipelines * docs: add third-party library attributions to NOTICE file * ci: Use cache for release pipeline

Co-authored-by: TomaszBlaszczak <182767772+tomasz-blasz@users.noreply.github.com>

* test: Move utils into common place for UTs & CTs * build: Transport v23

Copilot

Pull request overview

Copilot reviewed 96 out of 97 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

include/firebolt/firebolt.h:141

The comment contains a duplicate declaration of virtual Accessibility::IAccessibility& AccessibilityInterface() = 0;. This method is already declared at line 85 and shouldn't be duplicated here. The comment structure is also malformed with the closing comment on line 140.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-15T14:03:10Z

test/component/deviceTest.cpp

+    ASSERT_TRUE(result) << "DeviceImpl::timeInActiveState() returned an error";
+    if (expectedValue.empty())
+    {
+        std::cout << "[ !!!      ] Expected is empty, recived: " << *result << std::endl;


The spelling of "recived" is incorrect and should be "received".

Copilot · 2025-12-15T14:03:10Z

test/utils.cpp

+{
+    // Wait for the event to be received or timeout after 5 seconds
+    std::unique_lock<std::mutex> lock(mtx);
+    if (cv.wait_for(lock, std::chrono::seconds(EventWaitTime), [&] { return eventReceived; }))


The variable EventWaitTime is used directly in the wait_for call but should be wrapped in std::chrono::seconds() for consistency. Line 29 defines EventWaitTime as std::chrono::seconds(2), but here it's used as if it needs to be converted again with std::chrono::seconds(EventWaitTime).

Copilot · 2025-12-15T14:03:11Z

test/component/deviceTest.cpp

+    ASSERT_TRUE(result) << "DeviceImpl::uptime() returned an error";
+    if (expectedValue.empty())
+    {
+        std::cout << "[ !!!      ] Expected is empty, recived: " << *result << std::endl;


The spelling of "recived" is incorrect and should be "received".

Copilot AI review requested due to automatic review settings December 11, 2025 15:20

Copilot started reviewing on behalf of tomasz-blasz December 11, 2025 15:21 View session

Copilot AI reviewed Dec 11, 2025

View reviewed changes

tomasz-blasz force-pushed the topic/client-from-firebolt-apis branch from 1ec5b50 to e8aba6f Compare December 11, 2025 15:58

Copilot AI review requested due to automatic review settings December 11, 2025 15:58

Copilot started reviewing on behalf of tomasz-blasz December 11, 2025 15:59 View session

Copilot AI reviewed Dec 11, 2025

View reviewed changes

tomasz-blasz force-pushed the topic/client-from-firebolt-apis branch 2 times, most recently from 2ac7307 to f751d0a Compare December 12, 2025 07:22

Copilot AI review requested due to automatic review settings December 12, 2025 07:22

Copilot started reviewing on behalf of tomasz-blasz December 12, 2025 07:22 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

tomasz-blasz force-pushed the topic/client-from-firebolt-apis branch 2 times, most recently from ff8c41b to ef5521a Compare December 12, 2025 08:34

Copilot AI review requested due to automatic review settings December 12, 2025 08:34

Copilot started reviewing on behalf of tomasz-blasz December 12, 2025 08:35 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

tomasz-blasz and others added 11 commits December 12, 2025 09:40

ci: Update CI/CD with a support for testing replacement of native SDK…

9674fae

…, fix failing jobs (#:412)

feat(cpp): Thunder removal (#:422)

3d22251

Add component tests (#:423)

6eb26ab

* feat: Add component tests for APIs except events * refactor: Refactor FindCurl.cmake

feat: thunder post removal (#:424)

5fddbb3

* build: Set project version from release pipeline or tag * style: change include * test: fix gtest_discover_tests * ci: Update ci pipelines * docs: add third-party library attributions to NOTICE file * ci: Use cache for release pipeline

build: Fix absolute paths (#:427)

b4c699b

feat(cpp): Change initialization of the transport (#:428)

834ad22

feat: Lifecycle 2.0 (#:432)

a6f3791

Co-authored-by: TomaszBlaszczak <182767772+tomasz-blasz@users.noreply.github.com>

feat(cpp): Compile all test apps at once (#:433)

024461b

fix(cpp): Use cmake way to control shared/static library (#:434)

cafa305

fix: Update Lifecycle 2.0 to schema, update UTs (#:435)

d1d7424

feat(cpp): Update to changes around config (#:436)

f791d74

damcav64 and others added 17 commits December 12, 2025 09:41

feat: Device.chipsetId (#:476)

c677889

feat: Display API (#:474)

d342ce8

feat: Localization API (#:477)

22b9ebf

test: Editorial

688fb30

feat: Use Transport v21

7c3c37a

feat: Remove not agreed APIs

976eec9

ci: Add test coverage

8768bda

ci: Make logical chain

cb4f3d7

test: Update payload to JSON-RPC compliant params with an array (#:484)

aec10c9

Sync JS and CPP components for tests (#:486)

44cc51a

ci: Extend scope of coverage report (#:485)

5ac502c

fix: Update internal type

caca985

build: Move headers as they are installed

f7633f4

feat: Use FetchContent for the Transport if not installed

1e4d869

build: Transport v22

a739b20

test: Extend tests with negative scenarios (#:480)

86b4407

feat: Transport v23

ff94826

* test: Move utils into common place for UTs & CTs * build: Transport v23

tomasz-blasz force-pushed the topic/next branch from a66ecb2 to 1bf8ffe Compare December 12, 2025 08:41

tomasz-blasz force-pushed the topic/client-from-firebolt-apis branch from ef5521a to 5f47134 Compare December 12, 2025 08:42

tomasz-blasz added 4 commits December 15, 2025 14:48

build: When tests enabled, link against compiled lib, not installed one

4256851

feat: Use Transport v24

3c0c825

ci: Re-enable CI workflow

b38d12d

ci: Update to changes in repository layout

c126c58

Copilot AI review requested due to automatic review settings December 15, 2025 13:48

tomasz-blasz force-pushed the topic/client-from-firebolt-apis branch from 5f47134 to c126c58 Compare December 15, 2025 13:48

Copilot started reviewing on behalf of tomasz-blasz December 15, 2025 13:57 View session

Copilot AI reviewed Dec 15, 2025

View reviewed changes

tomasz-blasz closed this Dec 16, 2025

tomasz-blasz deleted the topic/client-from-firebolt-apis branch December 16, 2025 16:12

github-actions bot locked and limited conversation to collaborators Dec 16, 2025

Conversation

tomasz-blasz commented Dec 11, 2025

Uh oh!

github-actions bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

damcav64 commented Dec 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Dec 11, 2025 •

edited

Loading